Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Full Keyboard Access for checkboxes, radios and popups #1697

Closed

Conversation

mrcarlberg
Copy link
Member

Full keyboard access can be turned on by setting the key CPFullKeyboardAccess to YES
in the Info.plist file or set it programmatic on CPApplication.
A theme state "focused" is added to allow controls to show when it is in focus.
A CPMatrix embryo is added to support full keyboard access for radios.
It is only supporting radio buttons and is very simple.
TODO: There are controls that don't support keyboard access.
TODO: Aristo theme need graphics to show the new "focused" theme state.

I have written a test application using a special theme for demonstration: http://mini.carlberg.org/dev/FullKeyboardAccess/index-debug.html

…ups.

Full keyboard access can be turned on by setting the key CPFullKeyboardAccess to YES
in the Info.plist file or set it programmatic on CPApplication.
A theme state "focused" is added to allow controls to show when it is in focus.
A CPMatrix embryo is added to support full keyboard access for radios.
It is only supporting radio buttons and is very simple.
TODO: There are controls that don't support keyboard access.
TODO: Aristo theme need graphics to show the new "focus" theme state.
@cappbot
Copy link

cappbot commented Nov 30, 2012

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

@boucher
Copy link
Member

boucher commented Nov 30, 2012

Seems like you should be able to move the pop up button up/down with just the arrow keys, without having to first open it.

@boucher
Copy link
Member

boucher commented Nov 30, 2012

Is Matrix added here because the radio group is not in the responder chain? It seems unfortunate to have to add this class just for keyboard support.

@mrcarlberg
Copy link
Member Author

I believe the radio group is a hack to make radios work in a very simple way. The real implementation should be a matrix. I think it is essential to make a real matrix and get rid of the radio group as soon as possible. At least if we are going to go the Cocoa path.

@tolmasky
Copy link
Member

First off, very cool that you got full keyboard access working. However, I don't think CPMatrix is necessary in doing this. As Ross described, we considered having this in the past (and even had CPCells to do it appropriately), but after careful study decided it was unecessary. You describe our solution as a hack, hopefully I can convince you otherwise.

First off, its important to understand that NSMatrix exists because of NSCell and vice versa. These two classes are inexorably linked (As I'll describe in a second), and unless you plan on re-implementing the (useless in web world) NSCell classes, I could equally describe you solution as a hack (under your definition of things not "going the Cocoa way") because its not a real NSMatrix implementation since its storing views instead of cells. In fact, your implementation seems to do nothing that NSMatrix does other than appropriately handle keyboard commands. It would appear that it would more properly be named CPKeyboardGroupingView or something (which who knows, might be the right way to go). But I get the suspiscion that you want this to be the first of many commits that eventually leads to a full blown NSMatrix implementation.

So, for a little background, NSMatrix/NSCell were specifically implemented purely for performance reasons in the 80s. They tried making a calculator app or something and having 9 buttons or whatever was way too slow so the actual hack was encapsulating them all under one view, an NSMatrix, which then had 9 cells that drew the buttons. It performed the task of 9 views, with the "overhead" of 1. NSTableView did something similar, having NSCell prototypes that it would draw as necessary. However, you paid a huge conceptual price. Today, even in the constrained mobile environment of iOS, cells and matrices are completely unnecessary, these performance problems no longer exist. So, while our usual course of action is to trust the Cocoa frameworks, we do not do so blindly and in this particular case, where it was obvious that this was a decision made purely for platform/performance reasons 20 years ago, we opted to go the route of making these views the way they would have been made had they been implemented today. Here are some downsides to relying on having things in a matrix:

  1. You constrain yourself to a strict grid layout. You now have to talk in terms of intercell spacing and making sure its long enough for the longest title, etc. Its much nicer to just place individual radio buttons arbitrarily and have IB guides line them up for you. If for whatever reason you ever want to put one radio slightly further away, or want to give one radio a bigger height since it has more text, its not a huge deal at all with our method.
  2. You either need to use views (meaning you still won't actually have gone "the Cocoa Way") or throw in cells which make no sense in Cappuccino since its not a drawn environment. We used to have cells, they were just as heavy as views, since we operate like iOS with every view having its own context.
  3. NSMatrix is an amalgamation of a lot of different things you might want to do with completely unrelated controls. What does selectedCell mean when you have a matrix of uneditable textfields or buttons? There is an answer, but its clearly a stretch brought about by wanting "one solution" for all the different scenarios. This solution is a) not ideal for radios because its not as clear as -selectedRadio or whatever, and it b) doesn't make sense with all sorts of other views you could put in there.

Instead, we realized that radios were a class that required a relationship with other radios. Instead of trying to abstract that to "what if other things were like that", we made a very specialized RadioGroup class that represented this relationship, without having to exist visually (like an NSMatrix does, which again is strange, a view that represents an abstract relationship).

I think we can easily get full keyboard access by just implementing the same nsmatrix classes in the radio button, or worse case scenario, making RadioGroup a subclass of CPResponder and placing it in the responder chain, having it do the same thing your CPMatrix class is currently doing.

@tolmasky
Copy link
Member

BTW, I looked up the discussion from when I committed this. It brings up a bunch of other issues that I had forgotten, if you're interested: https://groups.google.com/forum/?fromgroups=#!searchin/objectivej/CPRadioGroup/objectivej/fEHL2cvunqM/dmKCQC_f9gkJ

@boucher
Copy link
Member

boucher commented Nov 30, 2012

The matrix class exists largely because of the necessity of cells in Cocoa,
something Cappuccino does not use. It was a pretty carefully considered
decision. Perhaps @tolmasky could offer his thoughts.

On Fri, Nov 30, 2012 at 11:37 AM, mrcarlberg notifications@github.comwrote:

I believe the radio group is a hack to make radios work in a very simple
way. The real implementation should be a matrix. I think it is essential to
make a real matrix and get rid of the radio group as soon as possible. At
least if we are going to go the Cocoa path.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1697#issuecomment-10901033.

@tolmasky
Copy link
Member

EDIT: "aren't completely unnecessary" to "are completely unnecessary"

@aparajita
Copy link
Contributor

I agree wholeheartedly. Feel free to implement CPMatrix. :-)

@aparajita
Copy link
Contributor

Even if a full NSMatrix is not necessary in Cappuccino, I think a case could be made for CPRadioGroup to at least be a CPView and for the radios to be subviews.

@mrcarlberg
Copy link
Member Author

Ok, you copy the objC language and make objJ. There are some tradeoffs because javascript is different from C. There has to be a line of compatibility that you draw based from these tradeoffs.
You copy the Cocoa frameworks from the Mac. There are some tradeoffs because Cappuccino is run in a browser and displays with html. There has to be a line of compatibility that you draw based from these tradeoffs.
I agree that the cells and the matrices from an old world.
But we kind of live in this world.
You guys have to decide where you draw the line of compatibility.

I have not said that we need to implement a full blown CPMatrix. But, I do think we need to get rid of the radio group and merge it into a CPMatrix that is a CPControl that groups the radios.
We are living with IB and I think it is better to implement CPMatrix and still live with IB then make our own IB.

@tolmasky
Copy link
Member

The reasoning against this was because it is an unnecessary constraint to force radio buttons to all exist under the same view. What if you want an interesting border view around each radio button? It's easy with the current method, you literally put each radio button in its own little parent view. This however could easily break an NSMatrix (which is why in Cocoa the way youd have to do this is subclass NSCell to make your fancy BorderedRadioButton).

So now you might suggest, "well, what if the Radio Container View was smart enough to traverse all subviews of subviews looking for radio buttons", OK, well now you have to listen to ANY subview change in ANY child of yours. Not to mention, One radio button could enable sub radio buttons, so you would also have handle the edge case of not traversing into other Radio Container Views.

The relationship between radio buttons is abstract, not visual. There is no reason to clutter your View Tree with views that exist solely for code-meaning vs. visual presentation. In the cases where you want all the radio under some common view, you can already do that. When you don't want them to, you can do that too.

@Me1000
Copy link
Contributor

Me1000 commented Nov 30, 2012

If CPRadioGroup were to be made a view, the it should simply subclass CPControl, override layoutSubviews, and add -addRadioWithTitle:, -radioWithTitle:, and -selectedRadio: Thus making it more like a popover like control, where you almost never interact with CPRadio itself.

That said, we get back to the world of being forced to line radios up in a grid. RadioGroups being a controller like object fixes that, you can position radios wherever you want, and a simple distance calculation would decide which control to highlight as you navigate with the keyboard…

@mrcarlberg
Copy link
Member Author

I agree that the relationship between radio buttons is abstract, not visual. But can you get IB to agree too?

@tolmasky
Copy link
Member

Are you having a specific issue with IB? Because we have code in nib2cib to convert NSMatrixes to CPRadioGroups, and its seemed to worked fine for lots of people so far. The point of my post was not to say "don't use IB". But if you are experiencing some bug, you should let us know so we can get to the root of your desire to have an NSMatrix (right now it seems to be a vague desire to have "the cocoa way" full realized). However, if you are experiencing an issue, we should definitely look into it. If the bug is just "we don't have full keyboard access", then I guarantee you we can make that work with the way things are set up now.

@boucher
Copy link
Member

boucher commented Nov 30, 2012

Radio groups already work correctly with interface builder, so how is that
an issue?

On Fri, Nov 30, 2012 at 1:43 PM, mrcarlberg notifications@github.comwrote:

Ok, you copy the objC language and make objJ. There are some tradeoffs
because javascript is different from C. There has to be a line of
compatibility that you draw based from these tradeoffs.
You copy the Cocoa frameworks from the Mac. There are some tradeoffs
because Cappuccino is run in a browser and displays with html. There has to
be a line of compatibility that you draw based from these tradeoffs.
I agree that the cells and the matrices from an old world.
But we kind of live in this world.
You guys have to decide where you draw the line of compatibility.

I have not said that we need to implement a full blown CPMatrix. But, I do
think we need to get rid of the radio group and merge it into a CPMatrix
that is a CPControl that groups the radios.
We are living with IB and I think it is better to implement CPMatrix and
still live with IB then make our own IB.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1697#issuecomment-10905166.

@mrcarlberg
Copy link
Member Author

No, I use IB with matrixes and they are converted to a nice view with radio subviews, all connected to the same radio group. They do the job.

But, you talk about "constrain yourself to a strict grid layout". This is what IB do. Then you talk about relationship is abstract, not visual. That is why I ask if you can get IB to agree to both of these?

I fully understand your arguments and I do agree with most of them. I do also think you can make full keyboard access work with the current setup with radio groups. But the solution will be much cleaner if you have a control view that takes care of it. Mostly because IB has this visual relationship between radios and is constrained to a strict grid layout.

You guys has to decide where you want to draw the line. I think that adding support for full keyboard access is cleaner in CPMatrix than in CPRadio now when we use IB. If we made our own IB then another solution might work better.

Make the call!

@boucher
Copy link
Member

boucher commented Nov 30, 2012

In what way do you believe it will be cleaner? I guess I'm unclear -- what
feature of IB will not work, or will be harder to implement, or will be
even harder to conceptualize, if keyboard access is done through radio
groups instead of through a matrix.

Your arguments seem to be primarily just the restating of your opinion, but
without any evidence about the actual differences people writing programs
on top of Cappuccino would encounter. Some examples here would be extremely
helpful in understand your point of view.

On Fri, Nov 30, 2012 at 2:29 PM, mrcarlberg notifications@github.comwrote:

No, I use IB with matrixes and they are converted to a nice view with
radio subviews, all connected to the same radio group. They do the job.

But, you talk about "constrain yourself to a strict grid layout". This is
what IB dot. Then you talk about relationship is abstract, not visual. That
is why I ask if you can get IB to agree to both of these?

I fully understand your arguments and I do agree with most of them. I do
also think you can make full keyboard access work with the current setup
with radio groups. But the solution will be much cleaner if you have a
control view that takes care of it. Mostly because IB has this visual
relationship between radios and is constrained to a strict grid layout.

You guys has to decide where you want to draw the line. I think that
adding support for full keyboard access is cleaner in CPMatrix than in
CPRadio now when we use IB. If we made our own IB then another solution
might work better.

Make the call!


Reply to this email directly or view it on GitHubhttps://github.com//pull/1697#issuecomment-10906631.

@tolmasky
Copy link
Member

I guess I don't understand the "bug". You start by agreeing that the solution makes sense, that IB integration works fine, and that keyboard access can be fixed with the existing system... Then you claim that having CPMatrix would somehow be cleaner. But again, you've not explained HOW it would be cleaner, WHAT would that fix? You've provided no details -- for all I know you have really good reasons for wanting this but so far you've only talked about some aesthetic that you hold important, and provided no real issues.

Is your only issue that you can't make radio buttons the way we like them in IB? For starters, I think you can by taking a different button and changing it to radio and then making your own custom radio group object in the top level objects area (but I'm on my iPad so i can't check). But even if you can't, who cares? As you've described it works fine with IB, AND we get the added benefit of making it simpler to create radio buttons in code with our system (which is the part of the discussion that hasn't been touched yet -- it's crazy hard in cocoa to code generate radios, but super easy with our system).

Also, there is no "line" that we use to make these decisions. It's not a religious framework, we take things on a case by case basis. We do our best to replicate the cocoa experience AND work on the web. Sometimes we have to make compromises. If we really wanted to be cocoa faithful we'd make the entire page a huge canvas and draw everything, but this is impractical so we chose a more iOS like view system under the hood. What you aren't realizing is that the matrix situation is the same: there is no "clean" answer. If you made a matrix class (which again, we tried), you would have a completely different (and in my opinion worse) set of compromises to deal with. In my mind it is MUCH less clean to have a 2% implementation of CPMatrix which serves only to fool people into thinking that things work the same way as in cocoa when they don't. There are no cells, radio buttons are not just NSButtons with a custom image (like they ridiculously are in cocoa if you read that mailing list post I linked to). I prefer to provide an explicitly different system -- that to your own admission converts fine -- than to further the ruse.

I think I've provided ample detail into why we did what we did, if you'd like to continue the discussion, please provide specific use cases where this fails, and how having a CPMatrix class would help.

@mrcarlberg
Copy link
Member Author

Holy Shit what this turn you guys on!

I still can't figure out how to make the radios the way you like it in IB as a radio always create there own radio group unless you create them with a matrix. I think it is confusing for a person from the Cocoa world to understand things like why you substitute the matrix with the radio group when you do bindings to the matrix in IB. I think it can be confusing when people are used to a matrix and you have to learn about radio groups. Sure it works as usually if you just add the matrix with some radios in IB, but my opinion is that eventually you have to go down and figure out how it works under the hood. After one year with Cappuccino I think I have been digging around in almost every part of the Cappuccino code base.

It sure can be confusing if only 2% of CPMatrix is implemented too. But I have the opinion that it is cleaner doing it in a more Cocoa like way then you guys. I started implementing full keyboard access without the matrix but felt if was cleaner doing it with the matrix. I'm sorry if I can't make you guys understand me better but english is not my first language.

We had to have full keyboard access in a Cappuccino project and I implemented it. I decided to share my code and If you guys don't like it, so be it. Someone might implemented full keyboard access in the future that you can use.

Why don't you just say this is not the way we want to implement this. I'm ok with that. Someone has to make the decision.

I'm not stupid, I do understand what you guys are saying. I understand why you added radio groups. I just have another opinion that it is better to make it more Cocoa like and try avoid smart solutions that is different from Cocoa. I'm sure you could have added the radio group solution in a more matrix like way with a clear line on what is implemented of the CPMatrix and what is not, so people could easily understand what will work. That way you should have had a solution with something Cocoa people could easy understand and also maybe have your simpler radio group approach.

If you are doing a "copy" of the Cocoa frameworks I think you should go as far as you can to make it a "copy" and not add smart solutions as replacements if you don't have to.

@boucher
Copy link
Member

boucher commented Dec 3, 2012

Ok, sounds like we aren't going to move forward here.

@daboe01
Copy link
Contributor

daboe01 commented Apr 7, 2018

@mrcarlberg times have changed. i would love to have CPFullKeyboardAccess. could we please revisit this cool stuff?

@mrcarlberg
Copy link
Member Author

I think it is up to the community to decide if Full Keyboard Access should be merged or not. The current Radio Button implementation in Cappuccino removes the NSMatrix view and replaces it with a RadioGroup object that is not a view. My implementation will keep the NSMatrix view and still create the RadioGroup object. The NSMatrix view handles full keyboard access and the RadioGroup still handles that only one radio button is selected. You can make the case that this is not optimal as all this can be handled in one place along with all the stuff discussed above. I feel that it is better to have the functionality than to wait for the perfect implementation. And it has worked for almost 6 years without any major changes.

@didierkorthoudt
Copy link
Contributor

As this works and doesn't break anything, +1

@theredhead
Copy link
Contributor

+1: Jeff Raskin demands this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants